Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resultTransformer and resultComparator #446

Closed
wants to merge 3 commits into from

Conversation

nevir
Copy link
Contributor

@nevir nevir commented Jul 21, 2016

You can provide a resultTransformer when configuring ApolloClient to be able to modify query/mutation results immediately before they are returned to the application. Generally, transformers should avoid mutations of their input, as it will bust Apollo's result equality comparison (and cause unnecessary re-renders).

For cases where it is preferable to mutate the input (performance), you can also override how query results are compared for equality via resultComparator. This compares previously returned query results with newly fetched ones.

As an example, an application may wish to attach prototypes to result data (for read-only helpers) - it would need to modify the equality check to deep compare the two results while ignoring prototypes. If the newly returned result (without prototypes) matches the data present in the previous result (with prototypes), Apollo would consider it a no-op and not trigger any changes.


This is a continuation of #378

Also, note that the prototype example is how we are currently using this transformer in our application

@@ -249,7 +273,10 @@ export class QueryManager {
],
});

return result;
// data is optional in GraphQLResult, but not ApolloQueryResult.
const apolloResult = {data: result.data};
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear to me why TypeScript missed this previously (mutate has a return type of Promise<ApolloQueryResult>, and was previously resolving this promise with a GraphQLResult)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's probably because GraphQLResult is the same thing as an ApolloQueryResult but with an extra field tacked on.

Copy link
Contributor Author

@nevir nevir Jul 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The data property is optional in GraphQLResult, but required in ApolloQueryResult


Argument of type 'GraphQLResult' is not assignable to parameter of type '{ data: any; }'.
  Property 'data' is optional in type 'GraphQLResult' but required in type '{ data: any; }'.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lemme bump this to just cast the value rather than constructing a new object

@nevir nevir force-pushed the result-transformation branch 3 times, most recently from 5caf812 to de0f6a6 Compare July 21, 2016 22:25
nevir added a commit to convoyinc/apollo-client that referenced this pull request Jul 21, 2016
@stubailo
Copy link
Contributor

For cases where it is preferable to mutate the input

Is there ever a good reason to mutate it? I guess just to avoid cloning overhead?

@nevir
Copy link
Contributor Author

nevir commented Jul 22, 2016

Is there ever a good reason to mutate it? I guess just to avoid cloning overhead?

Yeah, I really worry about cloning the entire graph every time anything changes in the selection set

@igrayson igrayson force-pushed the result-transformation branch from de0f6a6 to 08e9367 Compare July 26, 2016 22:45
igrayson added a commit to convoyinc/apollo-client that referenced this pull request Jul 26, 2016
@nevir
Copy link
Contributor Author

nevir commented Aug 1, 2016

Hmm, AFAIK @igrayson did sign the CLA

@igrayson
Copy link
Contributor

igrayson commented Aug 7, 2016

Any feedback on this?

@stubailo
Copy link
Contributor

This looks fine to me! Sorry for taking a long time to look at it. If you rebase on top of current master I will merge it.

@nevir nevir force-pushed the result-transformation branch from 39d798f to 9fc6bc0 Compare August 11, 2016 23:11
@nevir
Copy link
Contributor Author

nevir commented Aug 11, 2016

@stubailo rebased!

@nevir
Copy link
Contributor Author

nevir commented Aug 15, 2016

@stubailo ping again on this :P

@igrayson
Copy link
Contributor

@stubailo another ping :>

nevir and others added 3 commits August 16, 2016 13:11
You can provide a `resultTransformer` when configuring `ApolloClient` to be able to modify query results immediately before they are returned to the application.  Generally, transformers should avoid mutations of their input, as it will bust Apollo's result equality comparsion (and cause unnecessary re-renders).

For cases where it is preferable to mutate the input, you can also override how query results are compared for equality.  As an example, an application may wish to attach prototypes to result data (for read-only helpers) - it would need to modify the equality check to deep compare the two results while ignoring prototypes.  If the newly returned result (without prototypes) matches the data present in the previous result (with prototypes), Apollo would consider it a no-op and not trigger any changes.
@nevir nevir force-pushed the result-transformation branch from a39c9bc to 90df07e Compare August 16, 2016 20:11
nevir added a commit to convoyinc/apollo-client that referenced this pull request Aug 16, 2016
…tion

`resultTransformer` and `resultComparator`
@igrayson
Copy link
Contributor

@stubailo ping

@stubailo
Copy link
Contributor

Merged! 🎉

Sorry it took so long.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants